Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Status from CoreData #1167

Merged
merged 63 commits into from
Jan 8, 2024
Merged

Remove Status from CoreData #1167

merged 63 commits into from
Jan 8, 2024

Conversation

kimar
Copy link
Contributor

@kimar kimar commented Nov 16, 2023 •
edited
Loading

Rationale

We've made the decision to radically decrease usage of Core Data as with the current implementation we're storing an excessive amount of data which leads to degraded performance and also invalid data over time, e.g. when account relations are changing but are not reflected in the App.

This PR removes usage of the CoreDataStack.Status entity and replaces it by MastodonStatus.

@kimar kimar changed the title [WIP] Start refactoring of Core Data to use MastodonSDK Entities [WIP] Start refactoring of Core Data to remove NSManagedObject<Status> Nov 22, 2023
@kimar kimar changed the base branch from develop to remove_staus_inital_draft November 22, 2023 11:47
@kimar kimar changed the base branch from remove_staus_inital_draft to develop November 22, 2023 11:47
@kimar kimar changed the title [WIP] Start refactoring of Core Data to remove NSManagedObject<Status> Remove Status from CoreData Nov 23, 2023
kimar added 17 commits November 27, 2023 11:18
@kimar
Fix handling of (re-)reblogged and favorites state in home timeline (I…
c7d62d5 
…OS-176)
@kimar
Fix manual refresh of home timeline not working (IOS-176)
4cb845e
@kimar
Fix duplicate entry after reblog (IOS-176)
faac99c
@kimar
Reimplement timeline anchoring (IOS-176)
9c5e193
@kimar
Merge branch 'develop' into remove_status
cc3f1d0
@kimar
Fix tabbar avatar not visible (IOS-176)
8316651
@kimar
Don't try to resolve self-relationship
a322a46
@kimar
Fix translate not working (IOS-176)
167e0e4
@kimar
Merge branch 'develop' into remove_status
03aeb1f
@kimar
Fix compile issues after resolving merge conflicts
9fed54d
@kimar
Fix CW cannot be hidden (IOS-176)
45accb2
@kimar
Fix bookmarked state (IOS-176)
7ad0b15
@kimar
Fix missing case in StatusFetchedResultsController
66b2bfd
@kimar
Improve StatusFetchedResultsController update handling (IOS-176)
1f73bee
@kimar
Improve flickering when interacting with posts (IOS-176)
112dce0
@kimar
Preserve isSensitiveToggled state when interacting with posts (IOS-176)
38423a1
@kimar
Fix sensitive toggled state for reblogs (IOS-176)
28cdd67
@kimar
Copy link
Contributor Author

kimar commented Dec 27, 2023

Testing results (WIP)

I just needed a place to write down findings so consider this a breathing list:

Good that you noticed this!

The crash happens because no user entity exists for this posts user anymore.

I just realized that there are still a couple of places like this where the MastodonUser needs to be replaced, however I'm not sure if this conflicts with your user refactoring.

@zeitschlag
Copy link
Contributor

however I'm not sure if this conflicts with your user refactoring.

I'm pretty sure it will, but so be it then. Also my domain-block-PR will create conflicts, but that's okay. I think it doesn't make sense to wait with those fixes until I'm finished with the MastodonUser-stuff

@kimar kimar requested a review from zeitschlag December 27, 2023 13:49
zeitschlag
zeitschlag reviewed Dec 28, 2023
View reviewed changes
Copy link
Contributor

@zeitschlag zeitschlag left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes since the last review look good, I like the UserIdentifier-approach. Can you say something about the other comments? Feel free to quietly resolve them :)

@kimar kimar requested a review from zeitschlag January 2, 2024 10:35
zeitschlag
zeitschlag reviewed Jan 4, 2024
View reviewed changes
Copy link
Contributor

@zeitschlag zeitschlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets better and better :) Great work!

I added some remarks and will do a test on device in the next few hours.

@zeitschlag zeitschlag self-requested a review January 8, 2024 09:06
zeitschlag
zeitschlag approved these changes Jan 8, 2024
View reviewed changes
Copy link
Contributor

@zeitschlag zeitschlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, finally! 🚀 👍 🎊 🥳

Thank you for that impressive amount of work.

@kimar kimar merged commit 976f934 into develop Jan 8, 2024
1 check passed
@kimar kimar deleted the remove_status branch January 8, 2024 10:17
@zeitschlag zeitschlag added this to the 2024.01 milestone Jan 9, 2024
@kimar kimar mentioned this pull request Jan 18, 2024
Merged
@zeitschlag zeitschlag restored the remove_status branch April 23, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zeitschlag zeitschlag zeitschlag approved these changes

Assignees

@kimar kimar

Labels
None yet
Projects
None yet
Milestone
2024.1
Development

Successfully merging this pull request may close these issues.
2 participants
@kimar @zeitschlag